fix: review follow-ups for tracel-runtime and tracel-job - #208

The crates' table tests move to rstest cases and fixtures, and the offline store checks its path names with relative-path instead of a hand-rolled check.

tracel-runtime declared its own JobDefinition with the same four fields
as tracel_job::JobDefinition, and the process adapter only converted one
into the other to build the command line and the definitions file. The
crate now re-exports tracel-job's type, and the conversion is gone.

The tests ran a command line through run_launched, a copy of execute, so
execute was never tested: that it writes the definitions before reading
the command line, reads TRACEL_JOB_NUM from the environment, fails with
exit code 1 when the definitions cannot be written, and succeeds after
printing. execute now takes the command-line arguments and the
environment lookup, Jobs::run gives it the process's own, and the tests
call it directly. The copy is gone.

The Jobs::prepare tests sat in execution.rs and the duplicate-name test
in the process adapter, away from the unit that decides them. They now
live in jobs.rs, around one rstest fixture of two jobs. The prepare test
no longer re-asserts the merge onto the default, which job.rs decides,
only that the prepared job carries its name and the input it runs with.
An unknown job and an input that does not decode are each refused with
their own PrepareError variant, the unknown job with the registered
names in order.

A panicked run's message is decided by catch_panic, and each thread
seeing its own scope by the scope module, which both test them; the
execution tests re-asserted them. They now check that a panic fails the
run, and the two-thread test is gone. The scope test asserted the number
and the input of two runs in one table; they are now two tests, the
number one rstest case per execution, each reading back what the job
saw.

The process tests re-asserted rules other units decide and test: the
merge onto the default (job.rs), the panic message (panics.rs), the
command line generated from the input schema (tracel-job), and the help
and version text clap prints. They now assert what the adapter maps: a
command line to the request it reads, the job and input to run or what
to print, and each way a run does not complete to its exit code, one
rstest case each. One test keeps that a job's schema reaches its flags.

The jobs are one rstest fixture, a job that completes and one that
fails, and record nothing: the tests compare the request a command line
gives, and errors by variant and fields, never by message. Test names
state the outcome.

The definitions test also read the file back through DefinitionsFile and
compared it with its own to_json, a round trip tracel-job owns and
tests. It now checks only the JSON describe writes. The failed-write
test checks the path the error holds instead of looking for the file
name in its message.

The test of an input without a default also checked what serde makes of
null, for a struct and for (), and the prepared input test compared
three prepared inputs, two of them with JSON written out by hand. The
first now checks only that the input decodes as given, and the second
that the prepared input is the JSON of what the job ran with. The inputs
that do not decode are rstest cases that check only that the input is
refused.

Two branches had no test: a scope entered over another restores it when
its guard drops, and write_atomically removes its temporary file when
the rename fails. The last test used to fail at the write instead; it
now renames onto an existing, non-empty directory.

RunnerEventStream gave a frame that did not decode as an error, and
serve_stream ended the session on any error, so one bad frame dropped
the connection and failed every running job. The stream now skips such
a frame with a warning and ends only when reading fails, which it logs;
it yields plain events, and serve_stream has no error to handle. The
stream is generic over BufRead, and its tests feed it bytes: a frame
whose data does not decode is skipped, and a failed read ends it.

The input decodes in Jobs::prepare, outside the catch_panic that
PreparedJob::run applies, so a panicking Deserialize unwound the job
thread before it reported its outcome or left the in-flight count:
wait_idle then blocked forever and the runner never reconnected. The
executor now catches a panic around prepare too and reports it as the
job's failure, and a dispatched job leaves the in-flight count through
a drop guard, whether its thread ends, unwinds or never starts.

The sibling crates deny missing docs; tracel-runtime did not, and the
variants and fields of RunnerError had none. The crate now denies them
too, and RunnerError says what each variant means.

run only checked that the URL parsed, so one with another scheme, or one
that cannot be a base such as `localhost:8000`, started the runner and
then panicked joining the endpoint path. run now rejects a URL that is
not http or https with RunnerError::NotHttp; any such URL can be a base,
so the joins cannot fail. The HTTP clients are built in run too, and a
client that cannot be built is RunnerError::HttpClient instead of a
panic.

The serve tests threaded a recording Vec of job numbers through every
test for the sake of one, waited on a Condvar with a ten-second timeout
to make two jobs meet, and re-asserted the panic message and each
thread's own scope, which panics.rs and scope.rs decide. The numbered
test now serves a job of its own that records the number it is given,
and the two concurrent jobs meet at a Barrier. How a job ended is one
rstest case per outcome, comparing the statuses reported rather than the
reasons, which are other units' error messages; one test compares a
failed job's whole finish, with the job's own error as its reason. Test
names state the outcome.

The protocol and SSE parser tests had given/when/then names, unlike the
rest of the crate; they now state what each one shows.

The crate keeps its reasoning in names and rustdoc. The two comments
left in serve.rs repeated what the warning and serve_forever's own
rustdoc already say, that a rejected finish is expected after a
disconnect and that running jobs drain before the runner reconnects.

The input schema is registered with the referencing crate, whose
registry failed to prepare when the schema referred to another document
or named a $schema it did not know, since its default retriever fetches
nothing, and the job then had no flags at all. The registry now has a
retriever that answers every such URI with the schema `true`, without
fetching it, and the schema is registered under a hierarchical base URI,
so a relative reference such as `shared.json` resolves too. Preparing
then succeeds. A field whose reference still does not resolve, because
it leads into a document that was not fetched, nowhere, or round in a
loop, takes a JSON literal, and every other field keeps its flag.

A bool flag took zero or one value, so in `<job> --shuffle '{"epochs":2}'`
the JSON document was read as the flag's value and rejected as an
invalid bool. A bool flag now takes a value only as `--shuffle=false`,
which is how tracel-cli forwards flags, so an argument after a bare
`--shuffle` is the job's JSON input.

Job subcommands never have a --version flag: only the program's
top-level command does, when it gives a version. Reserving the name
only took the flag away from an input field named `version`. Only
`help` and `config` are reserved now.

DefinitionsFile::new wrote tracel-job's own CARGO_PKG_VERSION as
sdk_version, which is documented as the version of the Tracel SDK the
program is built with. It now takes that version from its caller, and
tracel-runtime passes its own.

The protocol states that `jobs` is ordered by name, but
DefinitionsFile::new kept the order its caller gave. It now sorts them,
so every file it writes keeps the stated order.

It was documented as an example that is typically the default input,
but the command line always treats it as the default input: its values
are the flags' defaults and the input is merged onto it. The docs now
say so, as the crate docs already did.

A field's help fell back to the description of the type its $ref
points to, but not through the anyOf of that type and null that
schemars writes for an Option, so an undocumented `Option<Backend>`
had no help. The fallback now follows the optional's value as well.

The command tests built jobs through a Definition fixture that
duplicated JobDefinition and gave every job the same description, so
the subcommand test could not tell whose description it saw. The
fixtures now return JobDefinition with distinct descriptions, and the
test checks each job's own.

The flags tests renamed their Optimizer and Resume fixtures, which the
command tests define differently under the same names. Each test module
allowed dead code as a whole; the expectation now sits on the fixture
types that need it. Two command tests are renamed after what they
assert.

The command tests searched clap's rendered help and argument settings,
matched clap's error kinds and compared ConfigError messages. They now
check the input a command line gives: the field a flag sets and the
value it reads, that a required flag may be left to the JSON input or
the --config file, the order the layers win in, and the ConfigError
variant and path. The merge rules are checked on overlay itself, and
repeated cases are rstest cases.

Tests now show that an object that may be null takes a JSON literal, and that listed values leave out null, take JSON when one is not a scalar, and keep numbers as numbers.

local.rs ran to about a thousand lines doing six jobs. It is now
local/ with a module each for run directories and numbering, the
status file, the events line format, and artifacts, the session and
provider staying in mod.rs. The events line format is the file
contract other tools read, so it gets its own module and tests.

A pure move: no behaviour changes, and each test moves to the module
that owns what it checks.

save_artifact joined the name onto the run's artifacts directory
unchecked and removed whatever was there, so `..` deleted the run and
an absolute name deleted that path, since joining an absolute path
replaces the base. The reader had the same gap.

Both now read the name as a relative-path RelativePath and normalize
it, refusing a name that resolves to the artifacts directory itself or
leaves it, as `..` does. The directory is built with to_path, which
ignores a leading `/`, so an absolute name stays inside the run.
Nested names such as `checkpoints/best` stay allowed, as
tracel-artifact's own bundle paths allow them.

The reader refused only empty ids and ids holding a slash or a
backslash, so `.` and `..` named the experiment's own directory or the
runs directory above it. Experiment names were not checked at all, so
`..` or an absolute name put runs outside the runs directory and its
.gitignore.

Both are now read as a relative-path RelativePath that must be one
plain component, and the directory is joined from that component, so
a leading `/` cannot leave the parent. The store's docs say how both
experiment and artifact names are read. The rule is tested on names,
then once through each caller: create_experiment creates nothing for a
name it refuses, and the reader loads nothing through an id that
climbs out and back into a run.

The job number test asserted a number and a text in one test. Each is
now an rstest case of the rule that a job number is recorded as a
number when it is one.

The crate and scope docs said a thread has no job "in a plain binary or
a unit test", and FinishSink said it was split out so the loop is
testable with a fake. A public doc says what the code does: there is no
job outside one, and the sink reports job outcomes to the station.

The panic test compared each failure with the exact message catch_panic
words, all in one test. A panic now fails the run, a returned error is
the run's failure as it was returned, and the panic message is the text
the payload carries, a &str or a String, as rstest cases.

One test checked what the functions give both while a scope is entered
and once its guard drops; they are now two tests, the first comparing
with the scope itself rather than numbers written out again. The two
threads entering their own scopes now meet at a Barrier while both are
entered, so a scope shared between threads would fail the test, and the
held scope is compared with the scope it holds.

The test of how a job ends ran three jobs and asserted six things about
them, among them the failed job's exact message. A job that completes,
fails or panics is now an rstest case of one fixture, each checking the
status of its outcome; the error a job fails with is job.rs's to test.

The definition test spelled out the default as JSON and also restated
that a bare function has no description or default; it now compares
with the fixture's own default, and the bare case is gone. The merge
test builds the expected input from the default it was merged onto, the
failed job is checked by the error's type rather than its message, and
the schema is compared with the input type's own. The repeated runs
compare what each run was given instead of asserting in a loop, and the
tests that only sent an input through serde and back are gone.

The two write tests each checked the contents and that no temporary
file was left, the second by guessing the temporary file's name. The
contents written and the replaced file are now one test each, and no
temporary file is left is its own test, listing the directory as the
failed-rename test does.

parse kept the one doc comment left among the process test helpers; its
name and signature say what it reads.

The definitions test compared what describe writes with the file's
JSON, which serde decides. The process adapter's tests already show
that a launcher asking for the definitions gets them.

The test counted the finishes, then looked each one up in a loop. It
now compares the statuses reported with both jobs completed, as the
other serve tests do, and takes the setup from the fixture.

Two parser tests checked a frame field by field, one of them after
counting the frames. They now compare the whole frame the parser gives.

runs_are_numbered_from_one_under_the_experiment_name also checked
each run's directory, which the location test pins, so it now checks
the numbers alone. The test of a runs directory that already exists
takes its run from the local_run fixture.

the_status_follows_the_run_from_the_start_to_the_end pinned the file
twice, checked its timestamps against chrono's format, and checked a
temporary file that the rename never leaves. Keeping the start and
stamping the end are now checked on the status itself, and the store
writing a finished run as completed is tested with the session that
writes it.

a_run_records_each_event_as_a_line_of_json drove one run through every
kind of event and compared each line it read back, which is what serde
writes, so it is gone. The attribute lines, which the session writes as
the run starts, are pinned with the session, in order of their keys.

The activity tests checked each ending in a test of its own and
reached into events by position. Endings, of a guard and of a run, are
now rstest cases compared as status and message, which ActivityStatus
now derives PartialEq for. Progress updates are one rule tested with
and without a meter, and each test asserts what it is named for.

The tests of a guard failing with the panic that unwinds it and of
running an activity, once in lib.rs and the tracing tests, move here
beside the code that decides them; the ambient scope a run installs
is checked through the current experiment, without tracing.

watched_run_logs_panics_from_other_threads lived in lib.rs, checked
the panic hook's line format through the thread name, and asserted
both the error log and the flush, the second under a comment. Two
tests in panic_watch.rs now check one each: a watched run logs a panic
on another thread as an error ending with its message, and flushes
what it logged.

scoped_handle_inherits_attributes_and_call_site_wins asserted the
level, the message, an inherited attribute, an overridden one and a
third, two of them under comments. Which attribute wins is
LogRecord::inherit_attrs's rule, now tested in log.rs, and the lib.rs
test checks only that a log through a scoped handle inherits its
attributes.

The run tests reached into events by position, matched panic reasons
on the hook's format and thread name, and asserted several outcomes
at once through a helper that returned each log's level, message,
attribute count and activity. Each test now checks one outcome on the
mock_run fixture: telemetry through an activity is one case per kind,
a handle of a finished or dropped run is inactive and discards logs,
a cancelled run ends as it is ended, and a stop request is logged
once, as a message. run_context_records_events goes: every test that
reads a log back shows it.

The tests built their fixture by hand with Fixture::new: it is now an
rstest fixture. run_ends_the_experiment_by_what_its_code_returns is
one case per outcome and checks only how the experiment ended, not
that the output and the error pass through. The test that run runs
nothing when the experiment does not start no longer matches the
backend's message, and the current experiment test leaves the end of
the scope to the context tests.

The context tests asserted the whole stack of scopes in one test, the
current experiment before and after every await, and that none was
left on the test thread, each behind an expect message. Each test now
checks one rule through the current experiment's id: an inner scope
overrides the outer one, leaving it restores the outer one, leaving
the outer one first keeps the inner one, a handle enters its run on
another thread, and instrumented tasks keep their own experiment
across an await, as does one spawned in the current experiment.

The run control tests asserted both the run's token and an activity's
linked one, and a registered activity's cancel both returned true and
cancelled its token. Each outcome is now its own test, and whether
cancel_activity finds an activity is one rule with a case registered
and one unknown.

The test child guarded its own cancel, so it counted at most one
cancel whatever the token did, and the concurrent test slept so its
two cancels would race. The child now counts every cancel it is sent,
and the two threads cancel together behind a barrier. The tests are
named for the outcome, and the one restating that a cancelled token
is cancelled goes.

The tracing tests lived beside the span helpers, matched each log's
message, and checked a span field and an event field overriding it in
one test under a comment. The layer's rules, routing to the current
experiment or to one a span names, skipping events outside any, and
turning event and span fields into attributes with the event's
winning, are now tested in layer.rs, one rule each, counting the logs
routed. The span helpers keep a test each, through the public
tracing_span of a run and of an activity.

The progress tests read the started activities by position and
asserted every name, parent, meter and attribute at once, and the
endings test listed four statuses under a comment each. Each rule now
has its own test, finding activities by the names the test gives
them: how the training and evaluation activities nest, what their
meters count, the epoch an epoch activity numbers, and how a split
and the training end, one case per outcome.

A run carried a location, its console page or its offline directory,
so that a job's experiment could print where it recorded as it
started. The console page was guessed by taking `api/` off the API URL
and appending the web app's routes, after asking the console whether
the project's owner is a user or an organization, and the offline
directory is one the user chose. Nothing read the line, and tracel run
gives its own note when it falls back to recording offline. A run no
longer says where its backend keeps it, and ExperimentLocation,
ProjectScope::owner_kind and Experiments::on_start, which only the line
used, are gone.